On Thu, Jun 04, 2026 at 10:51:07AM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> `patch_delta()` takes the source and delta sizes by value and writes
> back the reconstructed target size through an `unsigned long *`.  That
> datatype cannot represent a value that exceeds 4 GiB on systems where
> `unsigned long` is 32-bit (notably 64-bit Windows builds), though, even
> though the delta encoding itself, the on-disk layout, and the in-memory
> buffers happily carry such sizes. A `size_t` companion to
> `get_delta_hdr_size()`, `get_delta_hdr_size_sz()`, was introduced in
> 17fa077596 (delta, packfile: use size_t for delta header sizes,
> 2026-05-08) precisely so that `patch_delta()` could be widened without
> changing the on-the-wire decoding helper's signature.
> 
> Widen `patch_delta()`'s three size parameters to `size_t` and switch
> its internal use of `get_delta_hdr_size()` to the `_sz` variant.
> Then propagate the wider type through the callers.

Does `get_delta_hdr_size()` have any remaining callers after this patch
series? I currently only spot two such callers, and you convert both of
them in this patch.

And can we reasonably add a test case that exercises this change?

> diff --git a/packfile.c b/packfile.c
> index 89366abfe3..e202f48837 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1964,10 +1964,8 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
>  			      (uintmax_t)curpos, p->pack_name);
>  			data = NULL;
>  		} else {
> -			unsigned long sz;
>  			data = patch_delta(base, base_size, delta_data,
> -					   delta_size, &sz);
> -			size = sz;
> +					   delta_size, &size);

Nice that we get rid of this awkward construct.

Patrick

On Thu, Jun 04, 2026 at 10:51:08AM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> `write_reuse_object()` learned to track its packed-object size as
> `size_t` in 606c192380 (odb, packfile: use size_t for streaming
> object sizes, 2026-05-08), but the comparison sink it feeds,
> `check_pack_inflate()`, still takes the expected decompressed size
> as `unsigned long`. The call site bridges the mismatch with
> `cast_size_t_to_ulong()`, which on Windows turns a >4 GiB object
> into an immediate die().
> 
> That function only uses `expect` once: as the right-hand side of a
> `stream.total_out == expect` equality test against zlib's counter.
> zlib's own `total_out` counter is `uLong` and is therefore still
> 32-bit-bound on Windows. Widening `expect` to `size_t` cannot fix that,
> but it is a strict improvement nonetheless: instead of dying outright,
> an oversized object now simply makes the equality fail and lets
> `write_reuse_object()` fall back to `write_no_reuse_object()`, which
> decompresses and re-deflates the content (and which the larger
> pack-objects widening series targets separately).

Hm. I wonder whether it's possible to reset `stream.total_out` on every
iteration and instead have a local `size_t` variable that we use to
track the total number of inflated bytes?

Patrick

On Thu, Jun 04, 2026 at 10:51:09AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 82bc6dcc00..3dff898c43 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1239,6 +1239,8 @@ static void *gfi_unpack_entry(
>  	unsigned long *sizep)
>  {
>  	enum object_type type;
> +	size_t size_st = 0;
> +	void *data;
>  	struct packed_git *p = all_packs[oe->pack_id];
>  	if (p == pack_data && p->pack_size < (pack_size + the_hash_algo->rawsz)) {
>  		/* The object is stored in the packfile we are writing to
> @@ -1260,7 +1262,10 @@ static void *gfi_unpack_entry(
>  		 */
>  		p->pack_size = pack_size + the_hash_algo->rawsz;
>  	}
> -	return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep);
> +	data = unpack_entry(the_repository, p, oe->idx.offset, &type, &size_st);
> +	if (sizep)
> +		*sizep = cast_size_t_to_ulong(size_st);
> +	return data;
>  }

Nit, please feel free to ignore: do we want to add a NEEDSWORK comment
here?

Patrick

On Thu, Jun 04, 2026 at 10:51:11AM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> When I started the transition from `unsigned long` to `size_t`, in the
> interest of keeping the patches reviewable, I introduced these calls to
> prevent data type narrowing from silently failing to handle large object
> sizes. I also introduced `*_sz()` variants that would allow most of the
> callers to keep using that `unsigned long` that the 90s kindly asked to
> be returned.
> 
> After the preceding commits, the only places that called the narrow
> wrappers either no longer exist or already use the `_sz` form
> internally, so the wrappers just narrow values back through
> `cast_size_t_to_ulong()` for no reason.
> 
> Drop them and rename the `_sz` variants back to the natural names.

Aha, so you already address my comment I had on one of the preceding
patches :)

Patrick

On Thu, Jun 04, 2026 at 10:51:12AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index fa45f774d7..fa6e396ddc 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -120,7 +120,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  	struct object_id oid;
>  	enum object_type type;
>  	char *buf;
> -	unsigned long size;
> +	size_t size;
>  	struct object_context obj_context = {0};
>  	struct object_info oi = OBJECT_INFO_INIT;
>  	unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
> @@ -166,7 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {
>  			size_t s = size;
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  
>  		printf("%"PRIuMAX"\n", (uintmax_t)size);

Can't we drop this local variable completely and instead supply `&size`
directly?

> @@ -219,7 +225,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (use_mailmap) {
>  			size_t s = size;
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  
>  		/* otherwise just spit out the data */
> @@ -266,7 +272,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (use_mailmap) {
>  			size_t s = size;
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  		break;
>  	}
> @@ -446,7 +455,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
>  		if (use_mailmap) {
>  			size_t s = size;
>  			contents = replace_idents_using_mailmap(contents, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  
>  		if (type != data->type)

Likewise for these three instances.

> @@ -555,7 +564,7 @@ static void batch_object_write(const char *obj_name,
>  			if (!buf)
>  				die(_("unable to read %s"), oid_to_hex(&data->oid));
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			data->size = cast_size_t_to_ulong(s);
> +			data->size = s;
>  
>  			free(buf);
>  		}

And I think this site here can be adapted, as well.

> diff --git a/diff.c b/diff.c
> index 5a584fa1d5..816b89dc6c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4594,8 +4594,9 @@ int diff_populate_filespec(struct repository *r,
>  		}
>  	}
>  	else {
> +		size_t size_st = 0;
>  		struct object_info info = {
> -			.sizep = &s->size
> +			.sizep = &size_st
>  		};
>  
>  		if (!(size_only || check_binary))
> @@ -4617,6 +4618,7 @@ int diff_populate_filespec(struct repository *r,
>  			die("unable to read %s", oid_to_hex(&s->oid));
>  
>  object_read:
> +		s->size = cast_size_t_to_ulong(size_st);
>  		if (size_only || check_binary) {
>  			if (size_only)
>  				return 0;
> @@ -4631,6 +4633,7 @@ object_read:
>  			if (odb_read_object_info_extended(r->objects, &s->oid, &info,
>  							  OBJECT_INFO_LOOKUP_REPLACE))
>  				die("unable to read %s", oid_to_hex(&s->oid));
> +			s->size = cast_size_t_to_ulong(size_st);
>  		}
>  		s->should_free = 1;
>  	}

The flow in this function is quite weird if you ask me, but that's a
preexisting issue. This does look correct to me, even if it's awkward.

Patrick